This PR is a draft: I need to write docs, cleanup the code and resolve the issue explained in the first message, but if people want to try it out, it should work nicely, though you'll need to explore with your LSPs and read the code to see what to set in themes to see it working.

There's historically been reluctance to implement LSP highlighting in Helix (for performance reasons IIUC), but this was proposed as a potential solution #5589 (comment)

Tree-sitter will be the default highlighter, while range-based LSP syntax highlighting will be merged asynchronously.

Does this PR implement this functionality? Or has the Helix team's stance on LSP highlighting changed?

This PR does not implement that feature, and AFAIK the stance hasn't changed. It's uncertain what kind of discussion might happen around this PR with respect to that.

This PR does not implement that feature

Yes it does, this PR used the tree sitter coloring by default, only adding the LSP colors when it got them

Anyway, I'm not interested in working on helix right now, I just forgot to close this one

Ah, good to know if you or anyone else picks this up then.

F# for example relies pretty heavily on the LSP to distinguish variables from functions as the only way to figure that out is with types. They look the same in declaration. It doesnt have to be 100% LSP highlights; this specific problem can be solved if i give the LSP every identifier token to classify.

let x: int = 10
let y: unit -> int = fun () -> 20
let z a b = a + b // z: 'a -> 'a -> 'a

I'll take a look at picking this up.

@kirawi depending on how busy you are. I’d love to help out with this one if possible.

I've been thinking about this a bit more, and I started having some doubts about the "start with tree-sitter then merge semantic highlighting when it's ready" approach.

Instead, why not simply have an option to toggle semantic highlighting? So people who only want tree-sitter don't pay for semantic highlighting, and people who only want semantic highlighting don't pay for tree-sitter.

In addition to avoid wasting compute power, I imagine this would also be easier to implement overall.

Thoughts @kirawi ?

we don't want multiple alternative syntax highlighting implementations in core.

If we are going to support semantic highlighting doingit asynchrounsly and only for a few selected scopes is the only thing we are going to accept. Every vscode extension has a basic regex based highlighting so that delays in the LSP highlighting (which is always computed async so often not in the same frame) don't cause parts of the text to be unhighlited.

Some of these issues can be solved using word-based range mapping like in #6447 but with lots of syntax highlighting ranges that quickly becomes a performance problem. Similarly low timeout requests of syntax highlights is pretty wasteful (since there is no way to only request some scopes) so there should be a a fairly high request timeout (which makes range mapping event more important).

So to get acceptable performance for core you will need to request highlights rarely and only track very few highlight spans inside the editor.

@kirawi you should likely base the highlight requests on #8021 (you want to be pretty granular here). The implementation should likely be quite similar to inlay hints so you could consider doing that in one PR.

@pascalkuthe

we don't want multiple alternative syntax highlighting implementations in core.

If we are going to support semantic highlighting doingit asynchrounsly and only for a few selected scopes is the only thing we are going to accept.

Can you expand on why that is? IMO that should be a decision left to the user based on how much they value semantic highlighting and how performant their machine is.

I think such an entrenched position on the side of the project should at least have clearly stated technical reasons.

(To be clear, I'm not trying to argue for a different position here, I just think the reasoning should be documented somewhere)

@pascalkuthe

we don't want multiple alternative syntax highlighting implementations in core.
If we are going to support semantic highlighting doingit asynchrounsly and only for a few selected scopes is the only thing we are going to accept.

Can you expand on why that is? IMO that should be a decision left to the user based on how much they value semantic highlighting and how fast their machine is.

I think such an entrenched position on the side of the project should at least have clearly stated technical reasons.

As @archseer said recently on matrix about opinions like this in general: just "adding an option to let the user decide" when we have made a different design decision (in this case using tree sitter) is not something we accept usually. From the user perspective it just "lets them do what they want" but for us it means reviewing and maintaining a bunch of extra code. Code that lives in helix core should be up to a certain quality (including performance) so adding a bunch of different alternatives is a lot of work.

The technical reason in this case: its an inefficient/slow protocol and requires additional regex highlighting (which we also don't want to add/maintain) to avoid UI flickering. Tree sitter is used for many additional things inside helix besides syntax highlighting so it's a core editor component. We don't want to add/maintain alternatives to core editor components.

This is documented in the original issue for semantic highlighting, by the comment from @archseer there.